fix: usable error when install-script deps are missing; prefer sha256sum#116
Conversation
…256sum Addresses #111. The script already checked for `tar curl gzip shasum` and exited if any was missing, but the error ("tool 'shasum' not found") gave the user no path to recovery. On Ubuntu 24.04 minimal images this was the *only* missing tool — `shasum` is a Perl utility shipped by default on macOS but not Linux. Linux has `sha256sum` (GNU coreutils) preinstalled instead. Changes: - Accept either `sha256sum` (preferred — preinstalled on most Linux distros) or `shasum` (preinstalled on macOS). Both produce/consume the same `<hex> <file>` format used in `checksums.txt`, so the verification step works unchanged with either. On Ubuntu 24.04 this removes the failure entirely. - Collect every missing required tool (tar, curl, gzip, plus a checksum tool if neither sha256sum nor shasum is present) before exiting, so users only have to install once and re-run. - Print per-tool install hints covering Debian/Ubuntu, RHEL/Fedora, Alpine, and macOS so users know exactly what package to install. No behavior change on systems that already have `shasum`: the script picks `sha256sum` if present, falls back to `shasum` otherwise. The CI matrix (ubuntu-22.04, ubuntu-24.04, macos-14, macos-15) all have at least one of the two preinstalled, so existing install-script tests keep passing.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the dbdeployer-install.sh script by improving dependency checking and checksum verification. It now collects all missing tools and provides platform-specific installation hints instead of exiting immediately. Additionally, it dynamically selects between sha256sum and shasum for integrity checks. Review feedback recommends replacing the custom path-checking function with the more robust command -v builtin and using Bash arrays for the checksum command to ensure proper argument handling and avoid word-splitting issues.
| for tool in tar curl gzip | ||
| do | ||
| found_tool=$(exists_in_path $tool) | ||
| if [ -z "$found_tool" ] | ||
| if [ -z "$(exists_in_path "$tool")" ] | ||
| then | ||
| echo "tool '$tool' not found" | ||
| exit 1 | ||
| missing_tools+=("$tool") | ||
| fi | ||
| done |
There was a problem hiding this comment.
The custom exists_in_path function (defined at line 49) is potentially fragile because it splits $PATH on spaces (line 51), which will fail if any directory in the path contains a space. Since this script is using Bash, it is more robust and idiomatic to use the command -v builtin to check for the existence of executables.
| for tool in tar curl gzip | |
| do | |
| found_tool=$(exists_in_path $tool) | |
| if [ -z "$found_tool" ] | |
| if [ -z "$(exists_in_path "$tool")" ] | |
| then | |
| echo "tool '$tool' not found" | |
| exit 1 | |
| missing_tools+=("$tool") | |
| fi | |
| done | |
| for tool in tar curl gzip | |
| do | |
| if ! command -v "$tool" >/dev/null 2>&1 | |
| then | |
| missing_tools+=("$tool") | |
| fi | |
| done |
References
- Use the
command -vbuiltin instead of manual path searching to robustly check for executable existence in shell scripts.
| # Pick a checksum command: prefer sha256sum (Linux default), fall back to shasum (macOS default). | ||
| checksum_cmd="" | ||
| if [ -n "$(exists_in_path sha256sum)" ] | ||
| then | ||
| checksum_cmd="sha256sum -c -" | ||
| elif [ -n "$(exists_in_path shasum)" ] | ||
| then | ||
| checksum_cmd="shasum -a 256 -c -" | ||
| else | ||
| missing_tools+=("sha256sum-or-shasum") | ||
| fi |
There was a problem hiding this comment.
It is recommended to use a Bash array to store commands with their arguments. This avoids potential issues with word splitting and is considered a best practice for handling commands dynamically in Bash. Additionally, using command -v here is more robust than the custom exists_in_path function.
| # Pick a checksum command: prefer sha256sum (Linux default), fall back to shasum (macOS default). | |
| checksum_cmd="" | |
| if [ -n "$(exists_in_path sha256sum)" ] | |
| then | |
| checksum_cmd="sha256sum -c -" | |
| elif [ -n "$(exists_in_path shasum)" ] | |
| then | |
| checksum_cmd="shasum -a 256 -c -" | |
| else | |
| missing_tools+=("sha256sum-or-shasum") | |
| fi | |
| # Pick a checksum command: prefer sha256sum (Linux default), fall back to shasum (macOS default). | |
| checksum_cmd=() | |
| if command -v sha256sum >/dev/null 2>&1 | |
| then | |
| checksum_cmd=(sha256sum -c -) | |
| elif command -v shasum >/dev/null 2>&1 | |
| then | |
| checksum_cmd=(shasum -a 256 -c -) | |
| else | |
| missing_tools+=("sha256sum-or-shasum") | |
| fi |
References
- Use arrays to store commands and their arguments in Bash to avoid word splitting issues and improve maintainability.
| grep "$filename" "$checksum_file" | shasum -a 256 -c - | ||
| check_exit_code "shasum -c for $filename" | ||
| # $checksum_cmd was selected in STEP 2 — either `sha256sum -c -` or `shasum -a 256 -c -`. | ||
| grep "$filename" "$checksum_file" | $checksum_cmd |
There was a problem hiding this comment.
Execute the checksum_cmd array using the "${array[@]}" syntax to ensure arguments are handled correctly without being subject to word splitting.
| grep "$filename" "$checksum_file" | $checksum_cmd | |
| grep "$filename" "$checksum_file" | "${checksum_cmd[@]}" |
1 participant
Summary
Addresses #111. Three improvements to
scripts/dbdeployer-install.sh:sha256sum, fall back toshasum. The script previously requiredshasum(a Perl utility default on macOS, not on Linux). On Ubuntu 24.04 minimal images this was the only missing tool — butsha256sumfrom GNU coreutils is preinstalled there. Both tools produce/consume the same<hex> <filename>checksum format used inchecksums.txt, so the verification step works with either. This removes the failure mode entirely on Linux without forcing the user to install anything.tar,curl,gzip, and a checksum tool if neithersha256sumnorshasumis present) and print them all together before exiting.Before (issue #111 reproducer on Ubuntu 24.04)
After
On Ubuntu 24.04: succeeds —
sha256sumis preinstalled and used automatically.If a tool is missing, the user sees something like:
Test plan
bash -n scripts/dbdeployer-install.sh— syntax OKsha256sumpresent,shasummissing): script proceeds,checksum_cmdselectssha256sum -c -$PATH: all four tools reported together with hints, exit code 1shasum -a 256andsha256sumproduce a mutually-compatible<hex> <file>file format, sosha256sum -caccepts the existing releasechecksums.txtubuntu-22.04,ubuntu-24.04,macos-14,macos-15) all have at least one ofsha256sum/shasumpreinstalled, soinstall_script_test.ymlcontinues to passNotes
The change does not attempt distro auto-detection — keeping a
/etc/os-releaseparse out of the install script avoids adding maintenance burden the rest of the file doesn't have. The hints are short enough that users can self-select.